Skip to content

Kafka upgrade #3600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Oct 29, 2025
Merged

Kafka upgrade #3600

merged 6 commits into from
Oct 29, 2025

Conversation

@shroffk
Copy link
Member

@shroffk shroffk commented Oct 22, 2025 •
edited by kasemir
Loading

Checklist

  • Testing:

    • tested deploying on webpods and verified alarm servers default actions ( add/remove/modify configuration, generate alarms, ack/unack, enable/disable )

  • Documentation:

    • The feature is documented
    • The documentation is up to date
    • Release notes:
      • Added an entry if the change is breaking or significant
      • Added an entry when adding a new feature

@shroffk shroffk requested review from georgweiss and kasemir October 22, 2025 14:32
@shroffk
Copy link
Member Author

shroffk commented Oct 22, 2025

I have update the alarm server startup so that it will create and properly configure the kafka topics based on the config_name

It basically does that things the create_alarm_topics.sh does... with the added benefit that we don't need to remind anyone.

Note:
I have tested that it create new ones and fixes existing topics... but would prefer a bit more looking into

kasemir
kasemir approved these changes Oct 22, 2025
View reviewed changes
Copy link
Collaborator

@kasemir kasemir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!

@kasemir
Copy link
Collaborator

kasemir commented Oct 24, 2025

As for "The feature is documented", it's documented that you need to call create_alarm_topics.sh, which is no longer the case.
That script can be removed. The documentation should probably mention that we rely on a "compacting" topic for the alarm config and state, and how that's done automatically by running the alarm server for the first time on a new config.

georgweiss
georgweiss reviewed Oct 27, 2025
View reviewed changes
services/alarm-server/src/main/java/org/phoebus/applications/alarm/server/AlarmServerMain.java
"\trestart - Re-load alarm configuration and restart.\n" +
"\tshutdown - Shut alarm server down and exit.\n";

private static void ensureKafkaTopics(String server, String topic, String kafka_props_file) throws Exception {
Copy link
Collaborator

@georgweiss georgweiss Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The kafka_props_file argument is not used in this method.

georgweiss
georgweiss reviewed Oct 27, 2025
View reviewed changes
Copy link
Collaborator

@georgweiss georgweiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All seems to work as expected.

But I've added some comments.

@shroffk shroffk merged commit ded1e0b into master Oct 29, 2025
3 checks passed
@shroffk shroffk mentioned this pull request Oct 29, 2025
Closed
@smarsching smarsching mentioned this pull request Nov 8, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@georgweiss georgweiss georgweiss left review comments

@kasemir kasemir kasemir approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@shroffk @kasemir @georgweiss